Skip to content

[iOS] Fixes race condition when setup image loader#46153

Closed
zhongwuzw wants to merge 1 commit into
react:mainfrom
zhongwuzw:features/fix_image_loader_crash
Closed

[iOS] Fixes race condition when setup image loader#46153
zhongwuzw wants to merge 1 commit into
react:mainfrom
zhongwuzw:features/fix_image_loader_crash

Conversation

@zhongwuzw

Copy link
Copy Markdown
Contributor

Summary:

Fixes #46115 .

Changelog:

[IOS] [FIXED] - Fixes race condition when setup image loader

Test Plan:

crash in #46115

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Aug 22, 2024
@facebook-github-bot

Copy link
Copy Markdown
Contributor

@andrewdacenko has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Aug 23, 2024
@facebook-github-bot

Copy link
Copy Markdown
Contributor

@andrewdacenko merged this pull request in 6b104bb.

@react-native-bot

Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @zhongwuzw in 6b104bb

When will my fix make it into a release? | How to file a pick request?

meta-codesync Bot pushed a commit that referenced this pull request Jun 23, 2026
…erForData: (#57297)

Summary:
`imageURLLoaderForURL:` had a broken double-checked lock on `_loaders`: the outer `if (!_loaders)` guard and the `for` loop iteration both ran without `_loadersMutex`, while the assignment ran inside it. `imageDataDecoderForData:` had no lock at all on `_decoders`. Both methods write the ivar in two steps (raw list then sorted), so a concurrent reader could observe the intermediate value and iterate a concurrently-released array, crashing with `EXC_BAD_ACCESS` in `objc_release` on a GCD worker thread.

Verified with a reproducer — crashes 5/5 without the fix, 0/10 with it: https://github.com/mfazekas/rn-rctimageloader-dcl-repro

## Fix

Build the loader and decoder lists once in `setUp` (alongside the URL request queue), under `_setupLock` and gated by the existing `_didSetup` atomic flag. Every entry point calls `setUp` before reading the now write-once `_loaders` and `_decoders`, so a reader can never observe a half-built or concurrently-released array.

This replaces the broken double-checked lock where the outer `if (!_loaders)` guard and the `for` loop iteration ran outside the mutex.

## Changelog:

[iOS] [Fixed] - Fix a data race in `RCTImageLoader` loader and decoder lazy initialization that could crash with `EXC_BAD_ACCESS`

## Related

Fixes #57296
See also #46115, #46153

Pull Request resolved: #57297

Reviewed By: javache

Differential Revision: D109404243

Pulled By: fabriziocucci

fbshipit-source-id: 359d4c70e2cbe1bc70a7e6a1dadc3e3b89c59ff4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

iOS: RCTImageLoader is documented to be thread safe but the setUp code has a race

3 participants